Skip to content

NXP backend - added support for aten.conv2d using the new Neutron flow#19717

Open
novak-vaclav wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:feature/EIEX-861-add-conv2d-support-new-neutron-flow
Open

NXP backend - added support for aten.conv2d using the new Neutron flow#19717
novak-vaclav wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:feature/EIEX-861-add-conv2d-support-new-neutron-flow

Conversation

@novak-vaclav

@novak-vaclav novak-vaclav commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Added support for aten.conv2d using new Neutron flow.

Test plan

tests can be manually run using pytest -c /dev/null backends/nxp/tests/

cc @robert-kalmar @JakeStevens @digantdesai @rascani @MartinPavella

Copilot AI review requested due to automatic review settings May 21, 2026 08:04
@pytorch-bot

pytorch-bot Bot commented May 21, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19717

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 1 Cancelled Job, 2 Unclassified Failures

As of commit 735d329 with merge base 10bc51e (image):

NEW FAILURE - The following job has failed:

UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:

CANCELLED JOB - The following job was cancelled. Please retry:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 21, 2026
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "module: nxp"

@novak-vaclav

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: nxp"

@pytorch-bot pytorch-bot Bot added module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate labels May 21, 2026
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Please also review @MartinPavella @roman-janik-nxp @StrycekSimon. Thank you 😄

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds aten.conv2d delegation coverage for the NXP Neutron “new flow” and expands the NXP backend test suite to validate delegation behavior and (where possible) numerical correctness.

Changes:

  • Add a new-flow support-check path in the convolution converter and tighten failure behavior when group conv decomposition is missing.
  • Add extensive new pytest coverage for conv2d delegation/non-delegation scenarios under the new Neutron flow (including depthwise and edge/bounds cases, plus targeted xfails for known issues).
  • Improve NPU-vs-CPU mismatch reporting in the output comparator by including the max diff in the assertion message.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
backends/nxp/tests/model_output_comparator.py Improves assertion failure message for output mismatches.
backends/nxp/tests/ir/converter/node_converter/test_conv_converter.py Adds a large new test suite for conv2d under the new Neutron flow, including delegation checks and known-issue xfails.
backends/nxp/backend/ir/converter/node_converters/ops_converters/convolution_converter.py Introduces new-flow target support checks and adjusts convolution argument handling / group-conv behavior.
backends/nxp/aten_passes/split_group_convolution.py Updates group-conv splitting pass to handle optional bias and propagate FakeTensor meta safely.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 9cedbc4 to bc6ecdf Compare May 21, 2026 10:29
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Fixed the issues found by Copilot.

@MartinPavella MartinPavella left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will review the rest of test_conv_converter tomorrow.

Comment thread backends/nxp/tests/ir/converter/node_converter/test_conv_converter.py Outdated
Comment thread backends/nxp/tests/model_output_comparator.py
Comment thread backends/nxp/tests/ir/converter/node_converter/test_conv_converter.py Outdated
@MartinPavella

Copy link
Copy Markdown
Collaborator

@novak-vaclav please take a look at this issue: https://jira.sw.nxp.com/browse/EIEX-809
Is it relevant/applicable?

Copilot AI review requested due to automatic review settings June 3, 2026 15:50
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from bc6ecdf to e6db65f Compare June 3, 2026 15:50
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 3, 2026

Copy link
Copy Markdown

CLA Not Signed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

Comment thread backends/nxp/tests/model_output_comparator.py
Comment thread backends/nxp/aten_passes/split_group_convolution.py Outdated
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Fixed issues mentioned by @MartinPavella and updated the tests so they are based on Neutron SW 3.1.2.
@MartinPavella Do you need me to rebase it onto your Neutron SW 3.1.2 update branch?

The support for padding mode same / valid will be solved in a separate issue.

@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from e6db65f to 1f46333 Compare June 3, 2026 16:31
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Fixed issues mentioned by Copilot.

Note: I am not using the use_qat pytest fixture in test_conv_converter deliberately, because sometimes I need to mark a test configuration with xfail when qat=False and sometimes when qat=True

@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Now I see I will have to rebase onto the Neutron SW 3.1.2 branch, will do

@MartinPavella MartinPavella left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏻
I will approve after you rebase to the 3.1.2 PR

Copilot AI review requested due to automatic review settings June 8, 2026 11:45
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 1f46333 to ba6aee9 Compare June 8, 2026 11:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

backends/nxp/tests/ir/converter/node_converter/test_conv_converter.py:1

  • AllCloseOutputComparator is imported from executorch.backends.nxp.tests.nsys_testing, but in this PR other test modules import output comparators from executorch.backends.nxp.tests.model_output_comparator. If nsys_testing doesn’t define/re-export AllCloseOutputComparator, this will fail at import time. Import the comparator from its actual module (or re-export it explicitly in nsys_testing/a shared test utilities module) to keep imports consistent and avoid test collection errors.
    examples/nxp/setup.sh:1
  • The --index-url value is unquoted; if EIQ_PYPI_URL ever contains special characters (or is empty/oddly formatted), word-splitting can break the command. Quote the variable (\"${EIQ_PYPI_URL}\") to make the script more robust.

@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from ba6aee9 to 469260c Compare June 8, 2026 11:57
@novak-vaclav

Copy link
Copy Markdown
Contributor Author

Fixed the issues with not taking into account the transposed variant of conv2d. Now it should be fixed.
I also rebased onto the current main, updating the code to use Neutron 3.1.2

@MartinPavella MartinPavella left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Once tests pass, we can merge.

@MartinPavella

MartinPavella commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

@novak-vaclav please take a look at the failing linting and neutron job.

Copilot AI review requested due to automatic review settings June 10, 2026 13:37
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 469260c to c6029b3 Compare June 10, 2026 13:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Comment thread backends/nxp/aten_passes/split_group_convolution.py
Comment thread backends/nxp/tests/ir/converter/node_converter/test_conv_converter.py Outdated
Comment thread backends/nxp/tests/generic_tests/test_batch_norm_fusion.py Outdated
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from c6029b3 to 6e3f568 Compare June 10, 2026 13:48
Copilot AI review requested due to automatic review settings June 10, 2026 14:34
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 6e3f568 to 270b744 Compare June 10, 2026 14:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Comment thread backends/nxp/tests/model_output_comparator.py
Comment thread backends/nxp/aten_passes/split_group_convolution.py
Comment thread backends/nxp/tests/generic_tests/test_batch_norm_fusion.py Outdated
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 270b744 to 5a99b69 Compare June 10, 2026 15:12
Copilot AI review requested due to automatic review settings June 12, 2026 11:18
@novak-vaclav novak-vaclav force-pushed the feature/EIEX-861-add-conv2d-support-new-neutron-flow branch from 5a99b69 to 735d329 Compare June 12, 2026 11:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comment on lines 224 to +230
w_data = self._get_tensor_constant_from_node(w)
b_data = self._get_tensor_constant_from_node(b)
if w_data is None or b_data is None:
continue # Only the standard case with static weights and bias is supported.

with_bias = b is not None
# Only the standard case with static weights and static bias (or bias=False) is supported.
if w_data is None or (b_data is None and with_bias):
continue
Comment on lines +339 to 344
pytest.mark.xfail(
reason="AIR-14679: should start working after `conv2d` giving incorrect results is fixed by Neutron team.",
strict=True,
)

def test_nsys_inference__with_conv(self, mocker):
Comment on lines 470 to +473
elif conv_utils.group_conv_convertible_into_multiple_convolutions(
t_op, conv_params.groups
): # Convert to separated `Conv2D`.
t_op.builtin_options = conv_2d_options.Conv2D()

return conv_utils.create_separated_convolutions_based_on_group(
t_op,
conv_params,
self.builder,
self._convert_unpadded_2D,
conv_utils.conv_op_factory,
)
):
raise RuntimeError("NXP backend: Group convolution was not decomposed.")
).exported_program()

assert len(edge_program.graph.nodes) == 21
assert len(edge_program.graph.nodes) == 7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants